-
Notifications
You must be signed in to change notification settings - Fork 49
feat: introduce Codec entity #2577
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
size-limit report 📦
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a new Codec entity that combines the functionality of Encoder and Decoder to support the Send API's requirement for setting up subscriptions during message sending to detect Filter and Store message loss.
- Adds new
ICodecinterface that extends bothIEncoderandIDecoder - Creates
Codecclass implementation withcreateCodecfactory function - Marks existing
createEncoderandcreateDecodermethods as deprecated in favor of the new codec approach
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/interfaces/src/message.ts | Defines the new ICodec interface type |
| packages/interfaces/src/waku.ts | Adds CreateCodecParams type and createCodec method to IWaku interface, deprecates encoder/decoder methods |
| packages/core/src/lib/message/codec.ts | Implements the Codec class and createCodec factory function |
| packages/core/src/lib/message/constants.ts | Extracts shared constants OneMillion and Version |
| packages/core/src/lib/message/version_0.ts | Updates to use extracted constants |
| packages/core/src/lib/message/index.ts | Exports new codec functionality and constants |
| packages/core/src/index.ts | Exports codec types and factory function |
| packages/sdk/src/waku/waku.ts | Implements createCodec method in WakuNode class |
| packages/rln/src/message.ts | Updates to use extracted Version constant |
Comments suppressed due to low confidence (1)
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
It seems to me that you need to extend the Instead, you "just" need to be able to easily calculate message hash so you can look monitor whether a given message hash (of an outbound message) is being received via filter/store to do the p2p reliability logic. |
|
@fryorcraken good point, we indeed need only but, to accommodate for it in SDK we would need to expose it in and, separately, I think combining
and having that will help us avoid exposing |
adklempner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very nice
|
It is hard to value the merits of an API by just looking at it. Write a quick doc and mini app to demo so we can better judge. How does it look like if Alice and Bob are sending each other messages, and they are both using ECIES (Alice has Bob's pubkey, Bob has Alice's), using your codec? Edit: I see you crossed out "dogfooding". I would say dogfooding is mandatory when introducing a new API. |
|
agree, I have another PR with this entity there, will combine these PRs |
|
moving everything from here into main PR |
Problem / Description
For Send API that is part of Waku API to work we need a way to setup subscription when message is getting sent so that we could detect
FilterandStoremessage loss.For this
Encoderis not sufficient.Solution
We should introduce new entity instead of
EncoderandDecoderthat will combine their functionality and can be used inFilterandLightPushat the same time.Notes
Checklist
[ ] Code changes are covered by unit tests.[ ] Code changes are covered by e2e tests, if applicable.[ ] Dogfooding has been performed, if feasible.[ ] A test version has been published, if required.